fix: three hangs that wedged the MCP server while it looked healthy (v2.67.1) - #62

The symptom

An MCP server that looks perfectly healthy. Event loop alive and idle, nothing logged, nothing crashed — and every c3_* tool call dying at Claude Code's 120s timeout. Diagnosed with py-spy on a live wedged process (v2.67.0, the version installed to site-packages).

Three independent causes, all on background threads started during MCP startup.

1. collection.delete(where=...) never returns

cli/mcp_server.py spawns the c3-embed-index thread → EmbeddingIndex.build() → _remove_file_chunks() → self._collection.delete(where={"doc_id": doc_id}).

Evidence: two py-spy dumps four minutes apart with byte-identical frames at chromadb/api/rust.py (RustBindingsAPI._delete), 0.031s of CPU over 3s, and zero writes to chroma.sqlite3 in ten hours.

The file already had a get-ids-then-delete-by-ids fallback, and its comment already suspected the where-delete — but it sat behind except, and an except clause cannot catch a call that never comes back. It was unreachable by construction.

That fallback is now the only path. The metadata filtering moves to get(where=...), which does return; delete() is left with the one argument shape never seen to stall.

2. An unbounded lock turned one slow call into a dead server

build() took self._lock with a bare with and held it across the entire build loop, so the wedged delete parked every later caller behind it forever.

It now acquires with a timeout via _acquire_build_lock(), mirroring the _init_lock pattern that _ensure_ready already used correctly. On contention it logs once at WARNING and returns a degraded result carrying the normal stats shape, so the callers that read it with .get() defaults (cli/c3.py, cli/hub_server.py, services/subprojects.py, oracle/services/c3_bridge.py) keep working untouched. A redundant build is worth far less than a responsive server.

with self._lock: → try: … finally: self._lock.release(), with a test pinning that a mid-build exception still releases.

3. subprocess.run(timeout=...) hangs inside its own timeout handler

check_gemini / check_codex / check_claude passed stdin=DEVNULL and timeout=10 and hung anyway. On Windows, when the timeout fires, CPython's handler kills only the direct child and then calls process.communicate() a second time with no timeout (the _mswindows branch of run() in Lib/subprocess.py). That join never completes while a surviving grandchild still holds the stdout/stderr write-ends.

Observed wedging the c3-delegate-prewarm thread for 10h and leaking its two reader threads, so delegate health checks never completed and every first c3_agent call paid full preflight.

All three now go through _probe_cli_version(): Popen + a taskkill /T process-tree kill (the grandchild case CPython's bare kill() misses) + a bounded communicate() in finally.

tests/test_cli_smoke.py documented this exact footgun for test code back in 2.43.0 and called it "the repo convention". Production code now follows it.

What was actually verified vs. taken on trust

Verified empirically against the installed chromadb 1.5.6:

• get(where=...) does not hang — 0.003–0.234s across 100 / 16k / 33k chunks at 768 dims, single-threaded, 7-thread concurrent, and with a second process holding the same persist dir. The replacement path is safe and performs comparably to the call it replaces.
• The fixed EmbeddingIndex driven end to end against a real PersistentClient: initial build (300 chunks), targeted removal, incremental rebuild with one deleted and one edited file (no duplicate chunks), force rebuild, and semantic search. All pass.
• RustBindingsAPI._delete is confirmed by reading the installed source to be the frame the py-spy dumps point at.
• CPython's no-timeout second communicate() confirmed by reading the installed Lib/subprocess.py.

Taken on trust, and worth stating plainly: delete(where=...) could not be made to hang in isolation across five configurations (fresh small collection; 16k chunks at 768 dims; after a writer was SIGKILLed mid-write; under cross-process contention; under 7-way same-process thread contention). So the hang is condition-dependent, and swapping the call is not proven on its own to be the whole story.

That is precisely why fix (2) matters: the bounded lock is load-bearing, not merely defence-in-depth. Even if some future backend call stalls the same way, it can no longer take the server down with it.

Tests

22 of the 26 new tests fail against the pre-fix source, including the deadlock reproduction itself. The hang is simulated with a blocking fake collection joined with a bound in a daemon thread, so a regression fails fast instead of wedging pytest — same convention test_cli_smoke.py uses.

The 3 failures are pre-existing on a clean main and unrelated (enforcement-policy scope resolving to global instead of default): the tests do not isolate HOME, so a global ~/.c3 config on the dev box leaks in. They should pass on clean CI runners.

ruff check . clean.

Install

Version bumped 2.67.0 → 2.67.1 in both pyproject.toml and cli/c3.py (kept in sync for tests/test_version_sync.py).